Add a `Result::into_ok_or_err` method to extract a `T` from `Result<T, T>` #80572

thomcc · 2021-01-01T02:21:31Z

When updating code to handle the semi-recent deprecation of compare_and_swap in favor of compare_exchange, which returns Result<T, T>, I wanted this. I've also wanted it with code using slice::binary_search before.

The name (and perhaps the documentation) is the hardest part here, but this name seems consistent with the other Result methods, and equivalently memorable.

rust-highfive · 2021-01-01T02:21:34Z

r? @shepmaster

(rust-highfive has picked a reviewer for you, use r? to override)

library/core/src/result.rs

jyn514 · 2021-01-01T03:08:18Z

ok_or_err makes me think of ok_or(), something like fn ok_or_err(self, err: E) -> Self. Maybe into_inner() is a better name?

thomcc · 2021-01-01T03:19:55Z

I'm comfortable renaming if desired. I think ok_or_err is clearer than into_inner, but into_inner is pretty good too (better than most of the other suggestions I got, which were: merge, collapse, converge, either_value, unwrap_either, ...).

I don't care that much about the name so long as the functionality is available under some name though.

Another suggestion I got was implementing From<Result<T, T>> for (bool, T), which is probably possible, but I didn't like, mainly because that seems strictly less convenient. For example, in the cas=>cmpxchg transition, I'd need to make atom.compare_and_swap(a, b, <ordering>) into <(bool, T)>::from(atom.compare_exchange(a, b, <ordering0>, <ordering1>)).1, which is awkward.

thomcc · 2021-01-01T03:53:45Z

library/core/src/result.rs

+    /// cases where you don't care if the result was `Ok` or not.
+    ///
+    /// [`Atomic*::compare_exchange`]: crate::sync::atomic::AtomicBool::compare_exchange
+    /// [binary_search]: ../../std/primitive.slice.html#method.binary_search


Note that:

I can't use the fancy intradoc links stuff here for reasons that only @jyn514 comprehends.

Apparently primitives are only documented in std (No documentation for primitives in core #80334 or something), so I have to link it in std.

That said, I'm also okay with getting rid of these links entirely if that's desirable.

You can now use intra-doc links here!

Suggested change

/// [binary_search]: ../../std/primitive.slice.html#method.binary_search

/// [binary_search]: slice::binary_search

Actually, better yet you can change the part above that has the link to just [`slice::binary_search`] and remove this line.

You can now use intra-doc links here!

No, the bootstrap bump hasn't happened yet. Intra-doc pointers landed in #80181, which is in 1.51, which is still in nightly.

Dang, I always forget about the bootstrap bump :(

shepmaster · 2021-01-01T19:32:07Z

Maybe into_inner() is a better name?

This at least tracks with the usage in futures (and either).

shepmaster · 2021-01-01T19:33:49Z

library/core/src/result.rs

+    /// `Err`.
+    ///
+    /// This can be useful in conjunction with APIs such as
+    /// [`Atomic*::compare_exchange`], or [`slice::binary_search`][binary_search], but only in


Binary search is the primary reason I've wanted similar, but I've also seen people want to use it in procedural macros where you use a TokenStream for success and failure cases.

This seems reasonable to add as an unstable feature.

vandenheuvel · 2021-01-05T11:10:33Z

Naming consistency with into_ok seems logical, so I propose a name like into_*, such as into_either or into_ok_or_err.

bors · 2021-01-26T23:05:40Z

☔ The latest upstream changes (presumably #81417) made this pull request unmergeable. Please resolve the merge conflicts.

m-ou-se · 2021-02-17T09:03:34Z

r? @m-ou-se

m-ou-se · 2021-02-17T10:51:09Z

We briefly discussed this in the libs team meeting last week, and agreed this is a function we should have. There was some discussion on the name, but felt comfortable landing this as ok_or_err or into_ok_or_err and leaving the bikeshedding for later (before stabilization).

Can you open a tracking issue? Thanks!

thomcc · 2021-02-17T16:56:00Z

Result::into_ok existing (even as unstable) is a compelling reason to use into_ok_or_err, so I renamed it (for now, I guess).

m-ou-se · 2021-02-17T19:37:42Z

@bors r+ rollup

bors · 2021-02-17T19:37:43Z

📌 Commit 404da0b has been approved by m-ou-se

Rollup of 8 pull requests Successful merges: - rust-lang#77728 (Expose force_quotes on Windows.) - rust-lang#80572 (Add a `Result::into_ok_or_err` method to extract a `T` from `Result<T, T>`) - rust-lang#81860 (Fix SourceMap::start_point) - rust-lang#81869 (Simplify pattern grammar, improve or-pattern diagnostics) - rust-lang#81898 (Fix debug information for function arguments of type &str or slice.) - rust-lang#81972 (Placeholder lifetime error cleanup) - rust-lang#82007 (Implement reborrow for closure captures) - rust-lang#82021 (Spell out nested Self type in lint message) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned shepmaster Jan 1, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 1, 2021

thomcc commented Jan 1, 2021

View reviewed changes

library/core/src/result.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

jyn514 added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. A-result-option Area: Result and Option combinators labels Jan 1, 2021

thomcc commented Jan 1, 2021

View reviewed changes

shepmaster reviewed Jan 1, 2021

View reviewed changes

camelid added the S-waiting-on-bikeshed Status: Awaiting a decision on trivial things. label Jan 22, 2021

Dylan-DPC-zz added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 24, 2021

rust-highfive assigned m-ou-se and unassigned shepmaster Feb 17, 2021

thomcc added 3 commits February 17, 2021 08:51

Add a Result::ok_or_err method to extract a T from Result<T, T>

f688bee

Fix doc link for slice::binary_search

7d30366

Rename Result::ok_or_err to Result::into_ok_or_err

2711b01

thomcc force-pushed the ok_or_err branch from 2b890f9 to 2711b01 Compare February 17, 2021 16:55

thomcc mentioned this pull request Feb 17, 2021

Tracking Issue for Result::into_ok_or_err / feature(result_into_ok_or_err) #82223

Closed

5 tasks

Add link to tracking issue rust-lang#82223

404da0b

thomcc changed the title ~~Add a Result::ok_or_err method to extract a T from Result<T, T>~~ Add a ~~Result::ok_or_err~~ Result::into_ok_or_err method to extract a T from Result<T, T> Feb 17, 2021

thomcc changed the title ~~Add a ~~Result::ok_or_err~~ Result::into_ok_or_err method to extract a T from Result<T, T>~~ Add a Result::into_ok_or_err method to extract a T from Result<T, T> Feb 17, 2021

thomcc mentioned this pull request Feb 17, 2021

add Result::{value, into_value} #79315

Closed

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 17, 2021

Dylan-DPC-zz mentioned this pull request Feb 17, 2021

Rollup of 8 pull requests #82241

Merged

bors merged commit 40e3af5 into rust-lang:master Feb 18, 2021

rustbot added this to the 1.52.0 milestone Feb 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a `Result::into_ok_or_err` method to extract a `T` from `Result<T, T>` #80572

Add a `Result::into_ok_or_err` method to extract a `T` from `Result<T, T>` #80572

thomcc commented Jan 1, 2021 •

edited

Loading

rust-highfive commented Jan 1, 2021

This comment has been minimized.

jyn514 commented Jan 1, 2021

thomcc commented Jan 1, 2021

thomcc Jan 1, 2021

camelid Jan 22, 2021

camelid Jan 22, 2021

jyn514 Jan 22, 2021

camelid Jan 22, 2021

shepmaster commented Jan 1, 2021

shepmaster Jan 1, 2021

vandenheuvel commented Jan 5, 2021

bors commented Jan 26, 2021

m-ou-se commented Feb 17, 2021

m-ou-se commented Feb 17, 2021

thomcc commented Feb 17, 2021 •

edited

Loading

m-ou-se commented Feb 17, 2021

bors commented Feb 17, 2021

	/// [binary_search]: ../../std/primitive.slice.html#method.binary_search
	/// [binary_search]: slice::binary_search

Add a Result::into_ok_or_err method to extract a T from Result<T, T> #80572

Add a Result::into_ok_or_err method to extract a T from Result<T, T> #80572

Conversation

thomcc commented Jan 1, 2021 • edited Loading

rust-highfive commented Jan 1, 2021

This comment has been minimized.

jyn514 commented Jan 1, 2021

thomcc commented Jan 1, 2021

thomcc Jan 1, 2021

Choose a reason for hiding this comment

camelid Jan 22, 2021

Choose a reason for hiding this comment

camelid Jan 22, 2021

Choose a reason for hiding this comment

jyn514 Jan 22, 2021

Choose a reason for hiding this comment

camelid Jan 22, 2021

Choose a reason for hiding this comment

shepmaster commented Jan 1, 2021

shepmaster Jan 1, 2021

Choose a reason for hiding this comment

vandenheuvel commented Jan 5, 2021

bors commented Jan 26, 2021

m-ou-se commented Feb 17, 2021

m-ou-se commented Feb 17, 2021

thomcc commented Feb 17, 2021 • edited Loading

m-ou-se commented Feb 17, 2021

bors commented Feb 17, 2021

Add a `Result::into_ok_or_err` method to extract a `T` from `Result<T, T>` #80572

Add a `Result::into_ok_or_err` method to extract a `T` from `Result<T, T>` #80572

thomcc commented Jan 1, 2021 •

edited

Loading

thomcc commented Feb 17, 2021 •

edited

Loading